Skip to content

fix(plugin-approvals, service-automation): report a repairable strand, a dropped snapshot and a cascade-failed run apart — through a read-only engine member, not the wire (#15358 B′) - #16704

Merged
os-zhuang merged 4 commits into
mainfrom
claude/issue-15358-stranded-discriminator
Sep 8, 2026
Merged

fix(plugin-approvals, service-automation): report a repairable strand, a dropped snapshot and a cascade-failed run apart — through a read-only engine member, not the wire (#15358 B′)#16704
os-zhuang merged 4 commits into
mainfrom
claude/issue-15358-stranded-discriminator

Conversation

@os-trump

@os-trump os-trump commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Fixes #15358

Ruling implemented: B′ (director seat, decision batch #76, 2026-09-07T08:28Z, comment 5567692623; maintainer reply verbatim 「同意」). The discriminator is published as a dedicated read-only engine member, declared on ApprovalResumeSurface the way listSuspendedRunsDurable already is. The wire surface is untouched: no new key on ExecutionLogEntry, no packages/spec change, nothing added to GET /automation/:name/runs/:runId (PIN 3 of the reproduction now pins that). Reproduction base: #16459 (merged) — its PIN 1 went red on this tree and was flipped to the resolved expectation.

The defect

ApprovalService.classifyStrandedRunState keyed on run.status === 'failed'. A cascade-failed run — an ancestor failAncestors failed while parked at its subflow node, whose pause failSuspendedRun consumed and journalled nothing — has the same terminal failed row as the #13909 strand, so the inspection reported both as runState: 'failed' while restoreConsumedSuspension re-arms one and refuses the other (NO_CONSUMED_SUSPENSION). Open instance: #15222 remains open and is the first consumer.

What landed

@objectstack/service-automation (engine.ts, index.ts)

  • AutomationEngine.inspectConsumedSuspension(runId) — read-only. Answers from the SAME two witnesses restoreConsumedSuspension reads (this process's hot journal vs the durable row; rowSupersedesJournal, persisted, drop notice). The two-witness read is now one private method, resolveConsumedSuspensionWitnesses, called by both verbs, so they cannot disagree. The restore verb keeps the same decision table (its stale-hot-copy drop is now driven by the helper's staleHot flag) with one instant moved: the hot copy is now sampled after the loadTerminal await rather than before it, so a journal eviction (MAX_CONSUMED_SUSPENSIONS) landing during that await answers a refusal where the old read restored from the copy it had already captured — direction is refusal, never a double-run (the helper's TSDoc says so; contract-review item 1). Not a naive single loadTerminal read — ruling item 2 — and the ablation below shows the pins catch exactly that regression.
  • Answers, none folded: repairable: true (+ flowName / nodeId / correlation / witness / consumedAt), SNAPSHOT_DROPPED (+ the drop notice), NO_CONSUMED_SUSPENSION (cascade-failed, never paused, or a snapshot no longer held — the verb does not say which), RUN_SUSPENDED (+ nodeId). REJECTS when a store read fails — same posture as hasSuspendedRun. It re-arms nothing and writes nothing; its one incidental mutation is the strict suspension read's own phantom-entry eviction (service-automation: two loose ends on the #14333 claim path — the loser's hot-cache entry is never evicted, and the 'unsupported' branch answers only after its conditional delete has already been issued #15832, suspendedRuns only, never the journal) — identical to hasSuspendedRun (review item 7).
  • ConsumedSuspensionInspection exported from the barrel, for the same reason SuspensionRestoreResult is.

@objectstack/plugin-approvals (approval-service.ts)

Fail-closed on absence — the load-bearing property

Absence of the discriminator is NOT read as "not a strand". A surface that lacks inspectConsumedSuspension leaves a failed row reported as today's undifferentiated 'failed' — never 'unrepairable' (that calls the repairable row dead: the #15555 false-negative harm one surface over), never skipped (that hides the row) — exactly the ruling's item 1 fallback. A thrown read counts undetermined, like the other two oracles. Pinned twice: on the real engine (stranded-run-repairability.test.ts PIN 5 — the same two rows through a surface carrying only the two older oracles come back ['failed', 'failed']; re-attaching the full surface restores the split, as the positive control) and on a scripted surface (stranded-request-inspection.test.ts, the #15358 block: absent member, thrown read, unknown reason word, RUN_SUSPENDED skip, only-asked-for-failed-rows call count). An answer word this build does not know also stays 'failed'.

The two PM readings re-measured, with controls

Both readings in the dispatch (comment 5572530604, from 5568565582) were false at every tree measuredbc0ac1d1f (this branch's base), 8341ed21c (the base the dispatch named), and origin/main at ba5284e60:

  1. "consumedSuspensionDropped has zero hits repo-wide (no producer, no consumer, no test)". Measured git grep -n consumedSuspensionDropped18 hits in 7 files: declared on RunRecord (engine.ts:1331), produced by suspended-run-store.ts:796 (readConsumedSuspensionDropNotice), consumed by restoreConsumedSuspension (engine.ts drop-notice arm), tested in stranded-run-object-store.test.ts:297-300, exported type ConsumedSuspensionDropNotice in index.ts. Introduced by 5964124dd (feat(automation): stamp status: 'stranded' on the resume catch arm and pin the re-armed run's exactly-once — the #13937 services half (shape 4) #15237). Positive control: consumedSuspension → 29 hits in engine.ts alone. At 8341ed21c: git grep -c consumedSuspensionDropped 8341ed21c -- packages → 6 files.
  2. "The sentence quoted as the engine's own words is written in no file". git grep 'was not a strand' → 0 hits, but only because the sentence wraps: engine.ts:1323-1325 reads "a terminal record with neither field is a run that reached a terminal state which was not [newline] a strand (completed, cancelled, cascade-failed), or never paused at all." The sentence exists; the single-line grep was defeated by the line break.

Consequence, executed under the ruling's own words rather than the dispatch's reading of them: no field was built (the ruling's "one member" is the engine member, and there is one), but the existing consumedSuspensionDropped field is READ — ruling item 3 requires the snapshot-dropped class as its own member, and folding it either way is named there as the wrong shape. The contract review (PASS, zero blocking findings) judged the five-member StrandedRunState CONFORMS to the ruling's verbatim text.

Prose-only patch round (contract review items 1, 6, 7) — commit 42ce7bf4d

Three TSDoc edits and this body; no executable line changed, so the PASS against 1b4dad617 carries across. Proof: git diff 1b4dad617..HEAD -- '*.ts' → 36 changed lines, of which 0 are non-comment lines (every changed line, after stripping the sign and indentation, begins with a comment token); git diff --stat = approval-service.ts +7/−2, engine.ts +24/−3. Typecheck of both packages re-run green on the patch (check:test-typecheck: OK each; the plugin ledger unchanged). Review item 4 (refineFailedRunState running outside the try) is a behaviour change and is deliberately left exactly as it is, for the follow-up card.

Verification (all on 1b4dad617, this branch's head at review = merge of origin/main ba5284e60; the later 42ce7bf4d changes comment lines only)

  • pnpm --filter @objectstack/service-automation typecheck and ... plugin-approvals typecheck — both green (check:test-typecheck: OK for each; plugin debt ledger unchanged at 8 files / 324 / 27 pinned).
  • pnpm --filter @objectstack/service-automation testTest Files 125 passed (125) · Tests 1486 passed (1486); pnpm --filter @objectstack/plugin-approvals testTest Files 42 passed (42) · Tests 705 passed (705). Both packages' full suites; downstream importers owe no test (see consumers).
  • New / changed pins: packages/services/service-automation/src/consumed-suspension-inspection.test.ts (10 tests: journal and durable witnesses, RUN_SUSPENDED, never-paused, snapshot-cleared-after-finish, the dropped snapshot from the stranding replica vs a fresh one, both store outages reject, barrel nameability + exhaustive switch); stranded-run-repairability.test.ts PIN 1 flipped + PIN 5 added; stranded-request-inspection.test.ts [Decision] inspectStrandedRequests now over-reports: it keys on status === 'failed' while the platform gained an authoritative strand discriminator — a cascade-failed run the engine calls NOT stranded is reported as one #15358 block (10 tests).
  • Reverse verification (committed state, plugin repro via dist): PIN 1 with the pre-B′ expectation ['failed', 'failed'] re-applied → Tests 1 failed | 4 passed (5), AssertionError: expected [ [ …(2) ], …(1) ] to deeply equal …; restored with git checkout HEAD -- …, git hash-object = HEAD blob 613c8f6b… both sides, git diff HEAD --stat empty.
  • Ablation A — naive durable-only read (hot journal ignored in resolveConsumedSuspensionWitnesses): on-disk grep -c original line 1→0, marker 0→1; pnpm --filter @objectstack/service-automation build exit 0; ablation-dist-preflight "marker present in 2 built files"; engine pins Tests 2 failed | 8 passed (the journal pin and the "stranding replica still answers repairable from its hot copy" pin — the ruling's stop condition, caught). The plugin repro stayed green under this mutation by construction (its in-memory store row carries the snapshot), which is why the engine-side pins exist.
  • Ablation B — the member reads a held snapshot as absent (the service-automation: a throw between journalConsumedSuspension and the status: 'stranded' stamp makes a REPAIRABLE strand report repairable: false — the operator is told not to attempt a repair that works #15555 inversion, in the engine): marker a globalThis property so esbuild cannot fold it (a first attempt with a string-literal comparison was constant-folded out of dist and its reading discarded, as was an earlier non-compiling attempt); preflight "marker present in 2 built files"; engine pins Tests 3 failed | 7 passed; plugin Tests 2 failed | 35 passed — PIN 1 and PIN 5 red.
  • Restore leg for both: git checkout HEAD -- engine.ts, blob b00f251d… = HEAD, whole-tree git status --porcelain empty, rebuild, preflight --absent "marker absent from all 6 built files", engine 10/10 and plugin 37/37 green.
  • Derived gates: node scripts/pm/dispatch-gates.mjs --commands (no paths; merge-base self-derived, identical list pre- and post-merge) → 59 commands, all exit 0 after the packages closure was built; --ran → "59 derived famil(ies) accounted for — 59 run, 0 NOT-MEASURED". Three of them (check:dual-build-cjs-loads, check:i18n, check:type-check-debt) answered PREREQUISITE NOT MET (exit 3) before the closure build and green after it — not counted as red. pnpm check:error-code-casing (not derived; a new reason literal) green.
  • pnpm lint (repo-level eslint) not run locally — CI's Lint & Repo Gates.

Consumers of the changed exports (import edges, with a positive control)

Import edges of @objectstack/plugin-approvals outside the package (git grep "from '@objectstack/plugin-approvals" over packages/**, examples/**, apps/**): exactly oneexamples/app-showcase/test/approval-resume-relation-expand.test.ts:52 imports ApprovalsServicePlugin (the positive control). Zero import StrandedRunState, ApprovalResumeSurface or StrandedApprovalRequest; the remaining mentions are prose (a spec TSDoc, a CHANGELOG line, and automation-result-status.pin.test.ts asserting the docblock names StrandedRunState). The service-automation barrel addition is purely additive. ../objectui is not checked out in this container and was not measured.

Changeset

Judged against AGENTS.md's changeset rule: both packages publish, both diffs change published behaviour and published types → .changeset/stranded-run-state-discriminator.md, minor for each (additive members; the ruling names @objectstack/plugin-approvals minor). skip-changeset does not apply.

验收备注

  • Commit history: two wip: commits were pushed before the coordinator asked for real messages; rewriting pushed commits needs a force-push, which this lane forbids, so they stand (ruled). This PR title and body carry the real message; the prose patch commit 42ce7bf4d carries a real one.
  • noted, not filed: packages/spec/src/contracts/automation-service.ts:345 TSDoc says StrandedRunState is 'missing' | 'failed' — now stale prose; the ruling forbids a spec change in this PR, and it is a docs nit, not a defect.
  • noted, not filed: stranded-run-repairability.test.ts PIN 3 pins that getRun carries neither discriminator field — under B′ that is now the intended contract (wire untouched), so the pin's role flipped from "measurement" to "guard"; its docblock says so.
  • noted, not filed: inspectConsumedSuspension does not judge the restoring / resuming instant-safety guards (documented); a caller acting on repairable: true still goes through the restore verb, which does.
  • Contract review tier: CONTRACT_REVIEW_TIER read from scripts/pm/dispatch-gates.mjs = claude-fable-5-1. Clause-②: yesneeds:contract-review on issue and PR.

Attribution: authored in Claude Code session session_012zTkyNHJ7TkuN2oXtP5x37.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-approvals, @objectstack/service-automation, touching 26 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-automation/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via runId (symbol, a field of type ConsumedSuspensionInspection))
  • content/docs/automation/approvals.mdx (via nodeId (symbol, a field of type ConsumedSuspensionInspection), runId (symbol, a field of type ConsumedSuspensionInspection), /automation/:name/runs/:runId (route, a path literal in ApprovalResumeSurface; a path literal in AutomationEngine))
  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class), flowName (symbol, a field of type ConsumedSuspensionInspection), nodeId (symbol, a field of type ConsumedSuspensionInspection), runId (symbol, a field of type ConsumedSuspensionInspection), getRun (sdk, the bare tail of client method automation.getRun, bound to GET /automation/:name/runs/:runId), /automation/:name/runs/:runId (route, a path literal in ApprovalResumeSurface; a path literal in AutomationEngine))
  • content/docs/automation/jobs.mdx (via flowName (symbol, a field of type ConsumedSuspensionInspection))
  • content/docs/kernel/cluster.mdx (via nodeId (symbol, a field of type ConsumedSuspensionInspection))

2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via AutomationEngine (symbol, a top-level class))
  • content/docs/releases/v17.mdx (via AutomationEngine (symbol, a top-level class), flowName (symbol, a field of type ConsumedSuspensionInspection), nodeId (symbol, a field of type ConsumedSuspensionInspection), runId (symbol, a field of type ConsumedSuspensionInspection), getRun (sdk, the bare tail of client method automation.getRun, bound to GET /automation/:name/runs/:runId), /automation/:name/runs/:runId (route, a path literal in ApprovalResumeSurface; a path literal in AutomationEngine))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-automation/src/index.ts) — pages documenting those are invisible to this run
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 2e6a2ea4c90b2f1e8c86a2228d025e85be95836cpackageMentionDocs.

Which tree this was computed on

This run read content/docs from e62fa145519fc3a60c97e50225c08ad6d2bcc8c8 — the merge of head 42ce7bf4d847f098bb9e420ae1f94d6a3f7f3f74 into base 2e6a2ea4c90b2f1e8c86a2228d025e85be95836c, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin e62fa145519fc3a60c97e50225c08ad6d2bcc8c8 && git checkout e62fa145519fc3a60c97e50225c08ad6d2bcc8c8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2e6a2ea4c90b2f1e8c86a2228d025e85be95836c 42ce7bf4d847f098bb9e420ae1f94d6a3f7f3f74 && git checkout -B drift-repro 2e6a2ea4c90b2f1e8c86a2228d025e85be95836c && git merge --no-ff 42ce7bf4d847f098bb9e420ae1f94d6a3f7f3f74

node scripts/docs-audit/affected-docs.mjs --json 2e6a2ea4c90b2f1e8c86a2228d025e85be95836c

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 2e6a2ea4c90b2f1e8c86a2228d025e85be95836c → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…ampling instant, the strict read's one incidental eviction, and the third cause behind NO_CONSUMED_SUSPENSION

Prose-only patch round from the #15358 contract review (items 1, 6, 7): no executable line changes.

- resolveConsumedSuspensionWitnesses TSDoc: the hot copy is sampled after the caller's loadTerminal await, where the inline read it replaced sampled before it — same decision table, later instant; an eviction during that await answers a refusal, never a double-run.
- inspectConsumedSuspension TSDoc: 'drops nothing' softened — loadSuspendedRunStrict may evict a phantom suspendedRuns entry (#15832), identical to hasSuspendedRun; the journal is never touched.
- 'unrepairable' / NO_CONSUMED_SUSPENSION prose: names the third cause (snapshot no longer held) beside cascade-failed and never-paused.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
@os-zhuang
os-zhuang added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 4c31f02 Sep 8, 2026
38 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-15358-stranded-discriminator branch September 8, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

3 participants